Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

replace process exit with more detailed exit in src/bootstrap/*.rs #98994

Merged
merged 1 commit into from
Jul 7, 2022
Merged

replace process exit with more detailed exit in src/bootstrap/*.rs #98994

merged 1 commit into from
Jul 7, 2022

Conversation

kons-9
Copy link
Contributor

@kons-9 kons-9 commented Jul 7, 2022

Fixes #98830

I implemeted "detail_exit.rs" in lib.rs, and replace all of std::process::exit.
So, error code should panic in test code.

// lib.rs
pub fn detail_exit(code: i32) -> ! {
    // Successful exit
    if code == 0 {
        std::process::exit(0);
    }
    if cfg!(test) {
        panic!("status code: {}", code);
    } else {
        std::panic::resume_unwind(Box::new(code));
    }
}
% rg "exit\(" src/bootstrap/*.rs 
builder.rs
351:            crate::detail_exit(1);
1000:            crate::detail_exit(1);
1429:                    crate::detail_exit(1);

compile.rs
1331:        crate::detail_exit(1);

config.rs
818:                    crate::detail_exit(2);
1488:        crate::detail_exit(1);

flags.rs
263:                crate::detail_exit(exit_code);
349:            crate::detail_exit(exit_code);
381:            crate::detail_exit(1);
602:                        crate::detail_exit(1);
616:                crate::detail_exit(1);
807:            crate::detail_exit(1);

format.rs
35:            crate::detail_exit(1);
117:        crate::detail_exit(1);

lib.rs
714:            detail_exit(1);
1620:                detail_exit(1);
1651:pub fn detail_exit(code: i32) -> ! {
1654:        std::process::exit(0);

sanity.rs
107:            crate::detail_exit(1);

setup.rs
97:        crate::detail_exit(1);
290:            crate::detail_exit(1);

test.rs
676:            crate::detail_exit(1);
1024:                crate::detail_exit(1);
1254:            crate::detail_exit(1);

tool.rs
207:                crate::detail_exit(1);

toolstate.rs
96:    crate::detail_exit(3);
111:            crate::detail_exit(1);
182:            crate::detail_exit(1);
228:            crate::detail_exit(1);

util.rs
339:        crate::detail_exit(1);
378:        crate::detail_exit(1);
468:    crate::detail_exit(1);

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jyn514 (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2022
jyn514
jyn514 reviewed Jul 7, 2022
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is great! After you address the comments, can you squash the commits? There are instructions at https://rustc-dev-guide.rust-lang.org/git.html#advanced-rebasing.

src/bootstrap/lib.rs Outdated Show resolved Hide resolved
src/bootstrap/lib.rs Show resolved Hide resolved
@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 7, 2022
@jyn514
Copy link
Member

jyn514 commented Jul 7, 2022

@kons-9 I see you added a new "squash the commits" commit but that didn't actually squash them. After squashing you should have exactly one commit in the PR. Make sure you use git push --force instead of merging.

@kons-9
squash the commits
d6de276 
implement detail_exit but I'm not sure it is right.

not create new file and write detail exit in lib.rs

replace std::process::exit to detail_exit

that is not related to code runnning.

remove pub
@kons-9 kons-9 force-pushed the dont_use_process_exit branch from 93615e4 to d6de276 Compare July 7, 2022 04:43
@kons-9
Copy link
Contributor Author

kons-9 commented Jul 7, 2022

Sorry for the mishandling of the operation. I think this is probably done.

@jyn514
Copy link
Member

jyn514 commented Jul 7, 2022

@bors r+

Thanks for the PR!

@bors
Copy link
Contributor

bors commented Jul 7, 2022

📌 Commit d6de276 has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 7, 2022
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 7, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 7, 2022
@bors
Auto merge of rust-lang#99024 - matthiaskrgr:rollup-8ygpcpg, r=matthi…
1517f5d 
…askrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#97917 (Implement ExitCodeExt for Windows)
 - rust-lang#98844 (Reword comments and rename HIR visiting methods.)
 - rust-lang#98979 (interpret: use AllocRange in UninitByteAccess)
 - rust-lang#98986 (Fix missing word in comment)
 - rust-lang#98994 (replace process exit with more detailed exit in src/bootstrap/*.rs)
 - rust-lang#98995 (Add a test for rust-lang#80471)
 - rust-lang#99002 (suggest adding a derive for #[default] applied to variants)
 - rust-lang#99004 (Add a test for rust-lang#70408)
 - rust-lang#99017 (Replace boolean argument for print_where_clause with an enum to make code more clear)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6742dc4 into rust-lang:master Jul 7, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 7, 2022
@jyn514 jyn514 mentioned this pull request Aug 8, 2022
Closed
@shourya5 shourya5 mentioned this pull request Aug 9, 2022
Merged
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Aug 10, 2022
@compiler-errors
Rollup merge of rust-lang#100339 - shourya5:issue#100258, r=jyn514
f0fdc46 
Fixes bootstrap panic when running x fmt --check

closes rust-lang#100258 wherein bootstrap panics when running x fmt --check. Fixed by replacing resume_unwind  in rust-lang#98994. with process::exit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

Assignees

@jyn514 jyn514

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.

Don't use process::exit in bootstrap tests
5 participants
@kons-9 @rust-highfive @jyn514 @bors @rustbot